Conversation
| assert mock_tools.file.download.call_count == 0 | ||
|
|
||
|
|
||
| def test_existing_wix_install(mock_tools, tmp_path): |
There was a problem hiding this comment.
I've removed the remaining tests in this file because they were redundant:
- The fact that
upgradecallsinstallfollowed byunininstallis covered by tests/command/upgrade. - The functionality of WiX's
installanduninstallis covered by their own tests.
|
|
||
| ``[tool.briefcase.app.<app name>.<platform>.document_type.<document type id>]`` | ||
|
|
||
| The ``document type id`` is an identifier, in alphanumeric format. It is appended to the app id of an application to identify documents of the same type. |
There was a problem hiding this comment.
Removed internal implementation detail which is meaningless to the Briefcase user.
| The :attr:`extension` is the file extension to register. For example, ``myapp`` | ||
| could register as a handler for PNG image files by defining the configuration | ||
| section ``[tool.briefcase.app.myapp.document_type.png]``. |
There was a problem hiding this comment.
This implied that the section ID and the extension must be the same, or that the extension attribute is optional, neither of which are true.
| install_scope = "perMachine" if app.system_installer else "perUser" | ||
| except AttributeError: | ||
| # system_installer not defined in config; default to asking the user | ||
| install_scope = None | ||
| install_scope = "perUserOrMachine" |
There was a problem hiding this comment.
perUser, perMachine and perUserOrMachine are the 3 available scope keywords in the current version of WiX, and I had originally hoped to use them directly in the template. That ended up being more complicated for reasons that are explained in the template PR, but I've continued using the keywords in the Briefcase/template interface because it's a clear way of expressing Briefcase's intention.
freakboy3742
left a comment
There was a problem hiding this comment.
All looks good to me. Most of the complexity is in the templates; this is mostly a simplification of the downloading and invocation process - both of which are dramatically simplified because of the changes in WiX 5.
| extract_dir=os.fsdecode(self.wix_home), | ||
| self.tools.subprocess.run( | ||
| [ | ||
| "msiexec", |
There was a problem hiding this comment.
Are we certain we can rely on msiexec existing? And that it can be executed as a non-privileged user?
There was a problem hiding this comment.
Confirmed on my daughter's school-provided (and thus locked-down) windows device - msiexec is available, and can be executed by non-admin users.
|
One detail that occurred to me just after approving and kicking off the final build: WiX 3 required the user to provide an install .NET framework 3.5, which wasn't installed by default, so our docs include a mention of this. WiX now documents that all you need is "a text editor and the .NET SDK"... do we need to retain the .NET instructions? At the very least, we should update to point at a more recent SDK... but I think every win10 machine can guarantee the availability of .NET? Or am I mistaken? |
|
You're right, it looks like our current version of WiX requires .NET Framework 4.7 or later, and every supported version of Windows comes with 4.8 pre-installed. So I've just removed that section of the instructions. |
Corresponding template PRs:
Main changes:
<Files>element in the template. The "external app" mode therefore now only affectsbriefcase create, notbriefcase package.PR Checklist: